Skip to content

feat(simplex): treat an edited inbound message as a correction that supersedes the queued/in-flight original - #97317

Open
DavidMetcalfe wants to merge 4 commits into
NousResearch:mainfrom
DavidMetcalfe:feat/simplex-edit-supersede-35535
Open

DavidMetcalfe wants to merge 4 commits into
NousResearch:mainfrom
DavidMetcalfe:feat/simplex-edit-supersede-35535

Conversation

@DavidMetcalfe

Copy link
Copy Markdown

Summary

Implements edited-message supersede for the SimpleX gateway adapter, closing #35535.

Today the SimpleX adapter handles only newChatItems; a chatItemUpdated event (the terminal-API notification emitted when a user edits a sent message) falls into the unhandled-event log, so the agent acts on the original — mistaken — text. This PR treats an inbound edit as "I meant this instead", correlated to the specific original message by platform message id:

  • Original still queued (primary pending slot or overflow FIFO) → the queued MessageEvent's text is replaced in place. FIFO order, identity, and metadata are preserved; the correction is silent (no busy-ack).
  • Original in-flight (the turn's originating message id, now recorded on TurnState.active_message_id) → the running agent is redirected with the corrected text, framed as [User edited their earlier message. Corrected message: "..."]. Falls back to steer() when the agent doesn't support active-turn redirect, and to normal queueing when neither primitive is available. Neither primitive cancels in-flight tools — that is the best available behavior without mutating the transcript (see design notes).
  • Uncorrelated edits (no queued or in-flight match) → dropped with an info log rather than dispatched as a new message. Rationale in Open questions below.

Changes

plugins/platforms/simplex/adapter.py

  • _handle_chat_item now extracts meta.itemId and sets message_id=str(itemId) on every inbound MessageEvent (the prerequisite correlation key — previously never populated).
  • New chatItemUpdated branch routes the event's AChatItem through the same parse path with is_edit=True, tagging metadata={"is_edit": True}. The existing outgoing-direction guard means edits of the bot's own messages are ignored.

gateway/session_state.py

  • TurnState.active_message_id records the message id of the event that started the turn; cleared by the existing TurnState.clear() at every turn boundary (no new lifecycle to maintain, no leak path).

gateway/run.py

  • _replace_queued_message(): first-match, in-place text replacement across both queue levels, modeled on the existing _clear_goal_pending_continuations pattern.
  • _handle_edit_supersede(): the correlation decision — queued replace → in-flight redirect/steer → drop+log. Fully synchronous (no await between the queue check, the in-flight check, and the decision), so there is no interleaving window with queue promotion. Inserted in _handle_message after authorization, ignored-channel, and plugin-hook guards; adapter producers other than SimpleX get the same behavior free by tagging metadata["is_edit"] + message_id.

Design notes

  • Why not rewrite the transcript: the stale original text stays in history; per the project's prompt-caching constraint, past context is never mutated. The correction reaches the live turn via redirect/steer, mirroring existing mid-turn steering semantics.
  • Why gateway-core, not adapter-only: correlating an edit to the specific queued/in-flight original requires the queue/turn structures the adapter can't see — as the issue describes. Any future adapter that tags edits (Signal and Discord both surface them today in other forms) inherits the behavior.
  • Startup-resume path: the synthetic resume event carries no message id; its active_message_id stays None, so edits during a resumed turn fall to the uncorrelated policy rather than misfiring.

Open questions

  • Uncorrelated-edit policy: dropping (with a log) rather than queueing as a new message is a judgment call — Pro-side reasoning in the design review was that an isolated out-of-context correction ("the") injected into a fresh turn confuses the model; the counter-argument is that a drop silently loses user intent. If maintainers prefer "queue uncorrelated edits as normal messages", it's a two-line change in _handle_edit_supersede.

Tests

  • tests/gateway/test_simplex_plugin.py: 7 new tests — direct/group edit correlation, outgoing-direction suppression, message_id population on the normal path, image-content edits, missing-itemId fallback.
  • tests/gateway/test_edit_supersede.py (new): 15 tests — primary-slot replace, overflow-FIFO replace, in-flight redirect, steer fallback, sentinel-window handling, stale-correlation drop, uncorrelated drop, non-edit pass-through.
  • All 38 pass locally; the full tests/gateway/ suite shows only pre-existing environment flakes present on clean main.

Closes #35535

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 28, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Overall: Thoughtful edit-supersede implementation that treats a chatItemUpdated edit as a correction correlated by itemId/message_id. Queued text is replaced in-place (order preserved), in-flight turns are redirect/steer-framed, and uncorrelated edits are dropped — all synchronous on the gateway's single event loop with no await gaps (so no lock needed).

What it does

  • plugins/platforms/simplex/adapter.py:173 adds chatItemUpdated branch (stable itemId → message_id=str(itemId), metadata={"is_edit":True}), guards outgoing edits via existing direction check, and tags new newChatItems similarly; file-caption-only edit intentional.
  • gateway/session_state.py:73 adds TurnState.active_message_id (cleared with clear()).
  • gateway/run.py:9262 adds _replace_queued_message(session_key, adapter, message_id, new_text) (primary _pending_messages[session_key] then overflow FIFO queued_events, mutates .text in place, preserves FIFO) and _handle_edit_supersede(event, session_key, adapter) (edit+id guard, queued replace silent, in-flight active_message_id match → redirect(framed) → steer fallback, both best-effort, no tool cancellation, uncorrelated → info log + drop; explicitly notes single-loop synchrony avoids interleaving).
  • Tests tests/gateway/test_edit_supersede.py:234 and tests/gateway/test_simplex_plugin.py:411 cover queued primary/overflow replacement, no-match, in-flight redirect/steer, dispatcher wiring, and simplex direct/group/outgoing/message_id propagation.

Non-blocking notes

  • redirect vs steer is best-available: neither cancels in-flight tools — documented as without transcript mutation; mid-tool correction will interleave — acceptable tradeoff.
  • uncorrelated edit → dropped is deliberate to avoid out-of-context model confusion — ensure operator expectation documented (edit of already-consumed turn is not replayed).
  • Session-scoped correlation prevents cross-chat overwrite in groups (key derived from platform+chat_id) — correct.

Non-blocking — please use your judgment.

@DavidMetcalfe

Copy link
Copy Markdown
Author

@Enough1122 Thanks for the review.

One correction on the summary: the newChatItems path is not tagged as an edit — only the chatItemUpdated branch sets metadata={"is_edit": True} (plugins/platforms/simplex/adapter.py:676-677, gated on the is_edit parameter that branch passes); normal inbound messages gain only message_id=str(itemId). The gateway's supersede logic keys off is_edit + message_id, so ordinary messages are unaffected.

(Note: the review's line references had drifted — the chatItemUpdated branch is at adapter.py:453, TurnState.active_message_id at session_state.py:78, _replace_queued_message at run.py:9265.)

On the three non-blocking notes — all as designed: the redirect/steer no-tool-cancellation tradeoff is documented in the PR body's design notes, the uncorrelated-edit drop policy is under "Open questions" in the PR body awaiting maintainer preference, and group sessions are keyed per-chat so cross-user supersede can't happen.

@Enough1122

Copy link
Copy Markdown

Thanks for the correction — you're right. Re-reading at head b300eda6e9, only the chatItemUpdated branch sets metadata={"is_edit": True} (adapter.py:676-677, gated on is_edit), while newChatItems just gains message_id=str(itemId) — ordinary inbound messages are unaffected and the gateway's is_edit + message_id correlation is clean. My summary line conflated the two paths; noted and corrected.

The per-chat session keying and the documented redirect/steer and uncorrelated-drop tradeoffs are as you say — no action needed from me.

@DavidMetcalfe

Copy link
Copy Markdown
Author

@Enough1122 Appreciate the follow-up — glad the distinction is clear now. No further action needed on this thread.

@Enough1122

Copy link
Copy Markdown

Ack — correction noted and accepted. Thanks for clarifying the is_edit gating (only chatItemUpdated branch tags metadata={"is_edit": True}) and the documented tradeoffs. No further items from me.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(simplex): treat an edited inbound message as a correction that supersedes the in-flight/queued original

3 participants